fix(button): use kbd instead of span#1335
Conversation
✅ Deploy Preview for docsearch canceled.
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 4fd0905:
|
| <span className="DocSearch-Button-Keys"> | ||
| {key !== null && ( | ||
| <> | ||
| <span className="DocSearch-Button-Key"> | ||
| <kbd className="DocSearch-Button-Key"> | ||
| {key === ACTION_KEY_DEFAULT ? <ControlKeyIcon /> : key} | ||
| </span> | ||
| <span className="DocSearch-Button-Key">K</span> | ||
| </kbd> | ||
| <kbd className="DocSearch-Button-Key">K</kbd> | ||
| </> | ||
| )} | ||
| </span> |
There was a problem hiding this comment.
+1 for using semantic HTML. I'd also assume the check for key should happen one level higher as there's no point in rendering an empty span I'd imagine.
{key !== null && (
<span className="DocSearch-Button-Keys">
<kbd className="DocSearch-Button-Key">
{key === ACTION_KEY_DEFAULT ? 'Search' : key}
</kbd>
<kbd className="DocSearch-Button-Key">K</kbd>
</span>
)}There was a problem hiding this comment.
It was the case before, but we now use the first span to prevent UI flashes: #1033
shortcuts
left a comment
shortcuts
left a comment
There was a problem hiding this comment.
Thanks, good idea! I've also added it to the footer keys :)
| <span className="DocSearch-Button-Keys"> | ||
| {key !== null && ( | ||
| <> | ||
| <span className="DocSearch-Button-Key"> | ||
| <kbd className="DocSearch-Button-Key"> | ||
| {key === ACTION_KEY_DEFAULT ? <ControlKeyIcon /> : key} | ||
| </span> | ||
| <span className="DocSearch-Button-Key">K</span> | ||
| </kbd> | ||
| <kbd className="DocSearch-Button-Key">K</kbd> | ||
| </> | ||
| )} | ||
| </span> |
There was a problem hiding this comment.
It was the case before, but we now use the first span to prevent UI flashes: #1033
shortcuts
changed the title
|
@shortcuts Thank you for the update, but from the following changelog, I could not find which package is updated. That could be just why I am not sure how monorepo handles everything (and this kind of problems) at all 😉 https://github.com/algolia/docsearch/blob/HEAD/CHANGELOG.md#310-2022-05-18
|
I believe the confusion on your side is because how the DocSearch library is built. We only implement things in the React package, because the JS one is an alias from React to Preact and does not have any logic in it. So it's either a change on React, which is automatically reflected on JS, or a CSS change. |
|
Okay, I got the package structure now. Thank you for clarification. |
4 participants
Replaced span tag to kbd.